fix: message.max.bytes should default to 1048576 not 1 MB #2804

puellanivis · 2024-02-15T07:51:23Z

https://kafka.apache.org/documentation/#brokerconfigs_message.max.bytes

Says this default value should be 1048588, not 1 megabyte (and also weirdly not 1 Mebibyte 🤷‍♀️ ) matching these expected values is important, as one language could publish an event that another language would then refuse to republish.

The produced error message ConfigurationError(fmt.Sprintf("Attempt to produce message larger than configured Producer.MaxMessageBytes: %d > %d", size, p.conf.Producer.MaxMessageBytes) is also poorly designed to allow for event handling of that message. It would require an errors.As followed by a strings.HasPrefix(…) which is really not what Go recommends to handle errors (that is, direct string manipulation, which is inherently fragile as the error string should be easily changeable.) This change doesn’t really address this issue, but I wanted to mention it.

dnwe · 2024-02-21T09:48:45Z

@puellanivis thanks for pointing out this discrepancy with current versions of Kafka. However, it is slightly more nuanced as this value has had different default overs the years.

The Java producer since v0.8.2 has always had max.request.size of 1048576 (the base 2 version of 1 MB (MiB))

Up to Kafka v0.8.2 message.max.bytes was 1000000 server side
From Kafka v0.9.0 up to Kafka v2.4 it was then 1000012 (adding 12 bytes for log overhead from KAFKA-506)
From Kafka v2.5 onwards it has been a default 1048588 (the base 2 value of 1 MB (MiB) + 12 bytes for overhead again) based on KAFKA-4203 updating the broker config default to match the Java producer default

puellanivis · 2024-02-21T18:18:05Z

Yeah, I’m totally sure that it’s been through a lot of different values. 😰 Just thinking matching whatever is current is useful.

I suppose in our specific use case, we should probably set it to some value greater than whatever anyone would pick as a default. That way we can avoid “chasing a default value”.

dnwe · 2024-02-21T18:32:40Z

I think it probably makes sense for us to match the Java producer, which uses 1048576

puellanivis · 2024-02-21T21:34:58Z

I suppose I need to rebase and add sign offs into the commit anyways… actually, I’d probably be faster to just open a new PR with a signed commit message. 😂

Saram should match the max.request.size default from the Java producer config. https://kafka.apache.org/documentation/#producerconfigs_max.request.size Note that the server has its own limit on the record batch size which defaults to slightly higher than this, but is configurable and can also be set per topic too. https://kafka.apache.org/documentation/#brokerconfigs_message.max.bytes Co-authord-by: Cassondra Foesch <puellanivis@gmail.com> Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>

dnwe · 2024-02-22T23:13:04Z

No worries, I've done the rebase and updated the value to 1024*1024 as discussed

puellanivis · 2024-02-27T12:50:51Z

Is there anything blocking now?

dnwe · 2024-02-27T14:36:40Z

No further holdups, I was just waiting for you to reply with a thumbsup before I went ahead and merged 😄

puellanivis · 2024-02-27T21:56:25Z

😆 I guess in this case, I could have thumbs upped in a full comment rather than just as a reaction.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [github.com/IBM/sarama](https://github.com/IBM/sarama) | `v1.43.0` -> `v1.43.1` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fIBM%2fsarama/v1.43.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fIBM%2fsarama/v1.43.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fIBM%2fsarama/v1.43.0/v1.43.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fIBM%2fsarama/v1.43.0/v1.43.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>IBM/sarama (github.com/IBM/sarama)</summary> ### [`v1.43.1`](https://github.com/IBM/sarama/releases/tag/v1.43.1): Version 1.43.1 (2024-03-27) [Compare Source](https://github.com/IBM/sarama/compare/v1.43.0...v1.43.1)  #### What's Changed ##### 🐛 Fixes - fix: message.max.bytes should default to [`1048576`](https://github.com/IBM/sarama/commit/1048576) not 1 MB by [@puellanivis](https://github.com/puellanivis) in [https://github.com/IBM/sarama/pull/2804](https://github.com/IBM/sarama/pull/2804) - fix: add locking around broker throttle timer to prevent race condition by [@chengsha](https://github.com/chengsha) in [https://github.com/IBM/sarama/pull/2826](https://github.com/IBM/sarama/pull/2826) ##### 📦 Dependency updates - chore(deps): bump go.opentelemetry.io/otel/sdk from 1.23.1 to 1.24.0 in /examples/interceptors by [@dependabot](https://github.com/dependabot) in [https://github.com/IBM/sarama/pull/2816](https://github.com/IBM/sarama/pull/2816) - chore(deps): bump the golang-org-x group with 1 update by [@dependabot](https://github.com/dependabot) in [https://github.com/IBM/sarama/pull/2825](https://github.com/IBM/sarama/pull/2825) - chore(deps): bump github.com/stretchr/testify from 1.8.4 to 1.9.0 by [@dependabot](https://github.com/dependabot) in [https://github.com/IBM/sarama/pull/2822](https://github.com/IBM/sarama/pull/2822) - chore(deps): bump go.opentelemetry.io/otel/exporters/stdout/stdoutmetric from 1.23.1 to 1.24.0 in /examples/interceptors by [@dependabot](https://github.com/dependabot) in [https://github.com/IBM/sarama/pull/2815](https://github.com/IBM/sarama/pull/2815) #### New Contributors - [@chengsha](https://github.com/chengsha) made their first contribution in [https://github.com/IBM/sarama/pull/2826](https://github.com/IBM/sarama/pull/2826) **Full Changelog**: IBM/sarama@v1.43.0...v1.43.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib).  --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>

dnwe force-pushed the patch-2 branch from 66f344f to f4eb76d Compare February 22, 2024 23:09

dnwe changed the title ~~message.max.bytes should be 1048588 not 1 MB~~ fix: message.max.bytes should default to 1048576 not 1 MB Feb 22, 2024

dnwe force-pushed the patch-2 branch from f4eb76d to aa753c0 Compare February 22, 2024 23:11

dnwe added the fix label Feb 22, 2024

dnwe approved these changes Feb 27, 2024

View reviewed changes

dnwe merged commit 80b180c into IBM:main Feb 27, 2024
13 checks passed

puellanivis deleted the patch-2 branch February 27, 2024 21:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: message.max.bytes should default to 1048576 not 1 MB #2804

fix: message.max.bytes should default to 1048576 not 1 MB #2804

puellanivis commented Feb 15, 2024 •

edited

Loading

dnwe commented Feb 21, 2024

puellanivis commented Feb 21, 2024

dnwe commented Feb 21, 2024

puellanivis commented Feb 21, 2024

dnwe commented Feb 22, 2024

puellanivis commented Feb 27, 2024

dnwe commented Feb 27, 2024

puellanivis commented Feb 27, 2024

fix: message.max.bytes should default to 1048576 not 1 MB #2804

fix: message.max.bytes should default to 1048576 not 1 MB #2804

Conversation

puellanivis commented Feb 15, 2024 • edited Loading

dnwe commented Feb 21, 2024

puellanivis commented Feb 21, 2024

dnwe commented Feb 21, 2024

puellanivis commented Feb 21, 2024

dnwe commented Feb 22, 2024

puellanivis commented Feb 27, 2024

dnwe commented Feb 27, 2024

puellanivis commented Feb 27, 2024

puellanivis commented Feb 15, 2024 •

edited

Loading